Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Discard needless test cases in test_convolution_independent_gradients #15939

Merged
merged 3 commits into from
Aug 20, 2019
Merged

Discard needless test cases in test_convolution_independent_gradients #15939

merged 3 commits into from
Aug 20, 2019

Conversation

zixuanweeei
Copy link
Contributor

Description

test_operator.test_convolution_independent_gradients may cost too much occasionally, leading to terminated failed CIs. As it was discussed in ISSUE #15880, test_random.test_shuffle was also time-consuming, which has been already fixed by PR #15922. This PR aims to reduce the cost of test_convolution_independent_gradients by discarding some needless test cases. @pengzhao-intel @ciyongch @TaoLv

Checklist

Changes

  • num_bases change: [1, 16, 64] -> [1, 8]

Performance

Compiled with MKL-DNN Total test time / sec (bcbdc1c) Total test time / sec (b75357f)
N 41.968 16.133
Y 46.166 20.354

@pengzhao-intel
Copy link
Contributor

The runtime of MKLDNN is slower than the original build?

@zixuanweeei
Copy link
Contributor Author

Convolution with MKL-DNN contains the warm-up cost in test. So I think it might take more time to prepare memory and run into the convolution primitive.

@zixuanweeei
Copy link
Contributor Author

@pengzhao-intel CI has passed. Please take some reviews on it. Thanks.

Copy link
Contributor

@pengzhao-intel pengzhao-intel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and merging now.

@pengzhao-intel pengzhao-intel merged commit 3dfb19a into apache:master Aug 20, 2019
anirudhacharya pushed a commit to anirudhacharya/mxnet that referenced this pull request Aug 20, 2019
…s` (apache#15939)

* Drop needless num_bases in test of conv

* Trigger CI

* Re-trigger CI
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants